Deprecate the kibana reserved user; introduce kibana_system user #54967

legrego · 2020-04-08T16:54:52Z

Deprecates the reserved kibana user in favor of a new kibana_system user with an identical set of privileges.

This uses the same _deprecated and _deprecated_reason metadata attributes that we use to denote deprecated roles. Similarly, this logs a deprecation warning when a deprecated user successfully authenticates to Elasticsearch.

Partially addresses elastic/kibana#25879

Companion PRs:
elastic/stack-docs#991
elastic/kibana#63186

elasticmachine · 2020-04-08T19:50:17Z

Pinging @elastic/es-security (:Security/Security)

legrego · 2020-04-10T01:47:19Z

@elasticmachine merge upstream

legrego · 2020-04-10T19:59:33Z

I can't figure out why the SetupPasswordToolTests are failing. If anyone has an idea, I'd love to hear it. I can reproduce the failure locally, and removing my change from the SetupPasswordTool causes the tests to pass again, but I don't know what the actual problem is

legrego · 2020-04-10T20:17:50Z

...curity/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealmTests.java

@@ -125,10 +126,22 @@ public void testAuthenticationDisabledUserWithStoredPassword() throws Throwable
        verifySuccessfulAuthentication(false);
    }

+    public void testLogDeprecatedUser() throws Exception {


I don't love this test because it's fixed on the single deprecated kibana user, and won't work once we actually remove this user. I'm open to suggestions on how to rewrite this more generically, because it wasn't immediately obvious to me.

I could make the logDeprercatedUser method public so that it's testable on its own, but that didn't seem like the right solution either.

I think this is fine, but I would go with:

--- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealmTests.java @@ -126,16 +126,13 @@ public class ReservedRealmTests extends ESTestCase { verifySuccessfulAuthentication(false); } - public void testLogDeprecatedUser() throws Exception { - final User expected = new KibanaUser(true); - this.verifySuccessfulAuthenticationForUser(expected, true); - assertWarnings("The user [kibana] is deprecated and will be removed in a future version of Elasticsearch. " + - "Please use the [kibana_system] user instead."); - } - private void verifySuccessfulAuthentication(boolean enabled) throws Exception { final User expectedUser = randomReservedUser(enabled); this.verifySuccessfulAuthenticationForUser(expectedUser, enabled); + if (new KibanaUser(enabled).equals(expectedUser)) { + assertWarnings("The user [kibana] is deprecated and will be removed in a future version of Elasticsearch. " + + "Please use the [kibana_system] user instead."); + } } private void verifySuccessfulAuthenticationForUser(User expectedUser, boolean enabled) throws Exception {

albertzaharovits

Overall this looks OK.
But the setup password tool needs more thought. I suggest we prompt only once and set the input password for both users.

Also, docs need to be updated. Here's the list of files that refer to the kibana built-in user, and should instead refer to kibana_system:

x-pack/docs/en/security/authentication/built-in-users.asciidoc
x-pack/docs/en/security/auditing/output-logfile.asciidoc
x-pack/docs/en/security/get-started-kibana-users.asciidoc
x-pack/docs/en/security/get-started-builtin-users.asciidoc
x-pack/docs/en/security/auditing/output-logfile.asciidoc
x-pack/docs/en/security/securing-communications/tutorial-tls-internode.asciidoc

Assuming we plan to remove the kibana user in 8, we should drop a line about it in docs/reference/migration/migrate_8_0/security.asciidoc

albertzaharovits · 2020-04-15T13:26:00Z

...ty/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java

-    public static final List<String> USERS = asList(ElasticUser.NAME, APMSystemUser.NAME, KibanaUser.NAME, LogstashSystemUser.NAME,
-        BeatsSystemUser.NAME, RemoteMonitoringUser.NAME);
+    public static final List<String> USERS = asList(ElasticUser.NAME, APMSystemUser.NAME, KibanaUser.NAME, KibanaSystemUser.NAME,
+        LogstashSystemUser.NAME, BeatsSystemUser.NAME, RemoteMonitoringUser.NAME);



I think it's cumbersome on the user to have password prompts for both kibana and kibana_system.

I suggest we display the prompt for the new kibana_system user and also set the same password for the kibana user.

Good point, I agree this is cumbersome. I'll take a stab at updating the SetupPasswordTool to support this

albertzaharovits · 2020-04-15T14:15:30Z

...curity/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealmTests.java

@@ -125,10 +126,22 @@ public void testAuthenticationDisabledUserWithStoredPassword() throws Throwable
        verifySuccessfulAuthentication(false);
    }

+    public void testLogDeprecatedUser() throws Exception {


I think this is fine, but I would go with:

--- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealmTests.java @@ -126,16 +126,13 @@ public class ReservedRealmTests extends ESTestCase { verifySuccessfulAuthentication(false); } - public void testLogDeprecatedUser() throws Exception { - final User expected = new KibanaUser(true); - this.verifySuccessfulAuthenticationForUser(expected, true); - assertWarnings("The user [kibana] is deprecated and will be removed in a future version of Elasticsearch. " + - "Please use the [kibana_system] user instead."); - } - private void verifySuccessfulAuthentication(boolean enabled) throws Exception { final User expectedUser = randomReservedUser(enabled); this.verifySuccessfulAuthenticationForUser(expectedUser, enabled); + if (new KibanaUser(enabled).equals(expectedUser)) { + assertWarnings("The user [kibana] is deprecated and will be removed in a future version of Elasticsearch. " + + "Please use the [kibana_system] user instead."); + } } private void verifySuccessfulAuthenticationForUser(User expectedUser, boolean enabled) throws Exception {

…lasticsearch into security/deprecate-kibana-user

…ty/deprecate-kibana-user

legrego · 2020-04-23T17:36:57Z

@elasticmachine merge upstream

…ty/deprecate-kibana-user

…lasticsearch into security/deprecate-kibana-user

legrego · 2020-04-23T21:26:59Z

@albertzaharovits thanks for the review. I took a shot at your suggestion to automatically set the kibana user password based on what's generated for the kibana_system user. I'm running into an integration test failure because of the new deprecation warning I'm logging. I'm not sure of the best way to update the test. Do you have any suggestions?

https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+pull-request-2/22719/testReport/junit/org.elasticsearch.xpack.security.authc.esnative.tool/SetupPasswordToolIT/testSetupPasswordToolAutoSetup/

albertzaharovits · 2020-04-27T13:31:56Z

@elasticmachine update branch

albertzaharovits · 2020-04-27T14:32:26Z

@legrego

I'm running into an integration test failure because of the new deprecation warning I'm logging. I'm not sure of the best way to update the test. Do you have any suggestions?

This should do it.

diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/ClearRealmsCacheTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/ClearRealmsCacheTests.java
index 557fd5813bf..99df031c6df 100644
--- a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/ClearRealmsCacheTests.java
+++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/ClearRealmsCacheTests.java
@@ -11,6 +11,7 @@ import org.elasticsearch.action.support.PlainActionFuture;
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.RequestOptions;
 import org.elasticsearch.client.Response;
+import org.elasticsearch.client.WarningsHandler;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.settings.SecureString;
 import org.elasticsearch.test.SecurityIntegTestCase;
diff --git a/x-pack/qa/security-setup-password-tests/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolIT.java b/x-pack/qa/security-setup-password-tests/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolIT.java
index 48070300f0a..a5c1a9391cc 100644
--- a/x-pack/qa/security-setup-password-tests/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolIT.java
+++ b/x-pack/qa/security-setup-password-tests/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolIT.java
@@ -9,6 +9,7 @@ import org.elasticsearch.cli.MockTerminal;
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.RequestOptions;
 import org.elasticsearch.client.Response;
+import org.elasticsearch.client.WarningsHandler;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.io.PathUtils;
@@ -106,6 +107,10 @@ public class SetupPasswordToolIT extends ESRestTestCase {
                 Request request = new Request("GET", "/_security/_authenticate");
                 RequestOptions.Builder options = request.getOptions().toBuilder();
                 options.addHeader("Authorization", basicHeader);
+                if ("kibana".equals(entry.getKey())) {
+                    // the kibana user is deprecated so a warning header is expected
+                    options.setWarningsHandler(WarningsHandler.PERMISSIVE);
+                }
                 request.setOptions(options);
                 Map<String, Object> userInfoMap = entityAsMap(client().performRequest(request));
                 assertEquals(entry.getKey(), userInfoMap.get("username"));

The warning you get is an an _authenticate call used for verification that the tested tool did its job, i.e. the warning is not visible on a normal run of the command, it's present just in the test.

albertzaharovits · 2020-04-27T15:46:58Z

docs/reference/migration/migrate_8_0/security.asciidoc

+
+Users who were previously assigned the `kibana_user` role should instead be assigned
+the `kibana_admin` role. This role grants the same set of privileges as `kibana_user`, but has been
+renamed to better reflect its intended use.


This notice looks dope 👍

legrego · 2020-04-27T15:52:46Z

@albertzaharovits Thanks for the suggested edit for the integration test. I was initially considering something like that, but it felt a bit fragile, so I wasn't sure if there was a pattern used elsewhere that I wasn't aware of. I'll fix this test shortly and get a new version up here for re-review 👍

albertzaharovits

LGTM Thank you Larry!

We'll have to backport this to 7.x where the migration notice is in a different file, then follow-up with a master only PR that completely removes the kibana user.
You can fling this drudgery my way if you wish.

legrego · 2020-04-27T16:42:54Z

We'll have to backport this to 7.x where the migration notice is in a different file, then follow-up with a master only PR that completely removes the kibana user.

I don't mind taking this on, I'll bug you if I run into any issues :)
I won't merge the master-only PR until we have all the Kibana changes in place, that way we can continue to use updated snapshots as they become available

legrego force-pushed the security/deprecate-kibana-user branch from b974c0c to 601b05c Compare April 8, 2020 18:15

gwbrown added the :Security/Security Security issues without another label label Apr 8, 2020

legrego force-pushed the security/deprecate-kibana-user branch from 601b05c to 6813965 Compare April 8, 2020 20:36

deprecate the kibana reserved user; introduce kibana_system user

b34e4bb

legrego force-pushed the security/deprecate-kibana-user branch 2 times, most recently from 6e9c434 to ef8452a Compare April 9, 2020 14:17

fix license and test errors

7bc702d

legrego force-pushed the security/deprecate-kibana-user branch from ef8452a to 7bc702d Compare April 9, 2020 17:34

legrego added 2 commits April 9, 2020 14:50

fix IdentityProviderAuthenticationIT tests

b512555

test deprecation logging

761f401

This was referenced Apr 9, 2020

Deprecatekibana user in favor of kibana_system user elastic/stack-docs#991

Merged

Deprecate kibana user in favor of kibana_system user elastic/kibana#63186

Merged

Merge branch 'master' into security/deprecate-kibana-user

6b576e8

jkakavas requested a review from a team April 10, 2020 13:55

legrego commented Apr 10, 2020

View reviewed changes

legrego marked this pull request as ready for review April 10, 2020 20:21

legrego added v7.8.0 v8.0.0 labels Apr 10, 2020

albertzaharovits requested changes Apr 15, 2020

View reviewed changes

albertzaharovits added :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) >deprecation and removed :Security/Security Security issues without another label labels Apr 15, 2020

legrego added 5 commits April 17, 2020 12:02

First pass at SetupPasswordTool updates

1fe89d8

Merge branch 'security/deprecate-kibana-user' of github.com:legrego/e…

9cde855

…lasticsearch into security/deprecate-kibana-user

fix checkstyle

b32de55

update docs

56490e5

Merge branch 'master' of github.com:elastic/elasticsearch into securi…

f28670c

…ty/deprecate-kibana-user

elasticmachine and others added 4 commits April 23, 2020 13:37

Merge branch 'master' into security/deprecate-kibana-user

e92196c

Merge branch 'master' of github.com:elastic/elasticsearch into securi…

da5acfa

…ty/deprecate-kibana-user

Merge branch 'security/deprecate-kibana-user' of github.com:legrego/e…

d593899

…lasticsearch into security/deprecate-kibana-user

update number of expected users

50b58d4

Merge branch 'master' into security/deprecate-kibana-user

b9d1ab8

albertzaharovits reviewed Apr 27, 2020

View reviewed changes

albertzaharovits approved these changes Apr 27, 2020

View reviewed changes

update test to expect deprecation header

74fdc1c

legrego merged commit 269b152 into elastic:master Apr 27, 2020

legrego mentioned this pull request Apr 27, 2020

Backport: Deprecate the kibana reserved user (#54967) #55822

Merged

legrego added a commit that referenced this pull request Apr 28, 2020

Backport: Deprecate the kibana reserved user (#54967) (#55822)

47d2524

legrego mentioned this pull request Jun 3, 2020

Introduce kibana_system user, deprecate kibana user elastic/kibana#25879

Closed

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

masseyke mentioned this pull request Aug 13, 2021

Update Deprecation Info API checks for 8.0 #42404

Closed

80 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate the kibana reserved user; introduce kibana_system user #54967

Deprecate the kibana reserved user; introduce kibana_system user #54967

legrego commented Apr 8, 2020 •

edited

Loading

elasticmachine commented Apr 8, 2020

legrego commented Apr 10, 2020

legrego commented Apr 10, 2020

legrego Apr 10, 2020

albertzaharovits Apr 15, 2020

albertzaharovits left a comment •

edited

Loading

albertzaharovits Apr 15, 2020

legrego Apr 15, 2020

albertzaharovits Apr 15, 2020

legrego commented Apr 23, 2020

legrego commented Apr 23, 2020

albertzaharovits commented Apr 27, 2020

albertzaharovits commented Apr 27, 2020

albertzaharovits Apr 27, 2020

legrego commented Apr 27, 2020

albertzaharovits left a comment

legrego commented Apr 27, 2020

Deprecate the kibana reserved user; introduce kibana_system user #54967

Deprecate the kibana reserved user; introduce kibana_system user #54967

Conversation

legrego commented Apr 8, 2020 • edited Loading

elasticmachine commented Apr 8, 2020

legrego commented Apr 10, 2020

legrego commented Apr 10, 2020

legrego Apr 10, 2020

Choose a reason for hiding this comment

albertzaharovits Apr 15, 2020

Choose a reason for hiding this comment

albertzaharovits left a comment • edited Loading

Choose a reason for hiding this comment

albertzaharovits Apr 15, 2020

Choose a reason for hiding this comment

legrego Apr 15, 2020

Choose a reason for hiding this comment

albertzaharovits Apr 15, 2020

Choose a reason for hiding this comment

legrego commented Apr 23, 2020

legrego commented Apr 23, 2020

albertzaharovits commented Apr 27, 2020

albertzaharovits commented Apr 27, 2020

albertzaharovits Apr 27, 2020

Choose a reason for hiding this comment

legrego commented Apr 27, 2020

albertzaharovits left a comment

Choose a reason for hiding this comment

legrego commented Apr 27, 2020

legrego commented Apr 8, 2020 •

edited

Loading

albertzaharovits left a comment •

edited

Loading